Migrate from v1 to v2 of AWS SDK for Java #849
Conversation
adrianclay
force-pushed
the
upgrade-amazon-sdk
branch
from
9f29849 to
50bf772
Compare
v1 stops getting updates as of Dec 2025
adrianclay
force-pushed
the
upgrade-amazon-sdk
branch
from
50bf772 to
31c6457
Compare
| void expectToReturnS3ClientTest() { | ||
| var s3Client = configuration.getS3Client(); | ||
|
|
||
| assertNotNull(s3Client); |
There was a problem hiding this comment.
Ha. Hmm. Assert Not Null isn't the greatest test.
| @ConfigurationProperties(prefix = "gp2gp.storage") | ||
| public class StorageConnectorConfiguration { | ||
|
|
||
| private static final Region REGION = Region.EU_WEST_2; |
| Field s3ClientField = CustomTrustStore.class.getDeclaredField("s3Client"); | ||
| s3ClientField.setAccessible(true); | ||
| s3ClientField.set(customTrustStore, s3Client); |
There was a problem hiding this comment.
Personally not a fan of messing around with reflection to create tests. Any way to make the s3Client a constructor argument?
There was a problem hiding this comment.
Thinking about it, I'd probably make the AppInitializer construct it's own CustomTrustStore instead of relying on dependency injection. Also moving into AppInitializer the logic to optionally construct the S3Client that is currently inside of StorageConnectorConfiguration.
AppInitializer can instead depend on StorageConnectorConfiguration.
| this.s3client = AmazonS3ClientBuilder | ||
| .standard() | ||
| .build(); | ||
| this.s3client = s3client; |
There was a problem hiding this comment.
Will this use the Bean defined as getS3Client inside of StorageConnectorConfiguration.java ? Because if so, then that bean only gets constructed when a truststore is defined. Whereas previously this was being constructed all the time.
It will come from the StorageConnectorFactory which is fine.
.github/workflows/build_workflow.yml
Outdated
| ./gradlew test jacocoTestReport -x integrationTest --parallel --build-cache | ||
| else | ||
| ./gradlew test jacocoTestReport sonar --parallel --build-cache | ||
| ./gradlew test jacocoTestReport sonar -x integrationTest --parallel --build-cache |
There was a problem hiding this comment.
I'd move this stuff into a separate PR.
service/build.gradle
Outdated
| } | ||
|
|
||
| check.dependsOn integrationTest | ||
| jacocoTestReport.dependsOn integrationTest |
There was a problem hiding this comment.
Is this not what's causing the integration tests to run as part of the unit tests?
|
|
2 participants
Why
v1 stops getting updates as of Dec 2025
Type of change
Internal change (non-breaking change with no effect on the functionality affecting end users)
Checklist: